tests: restore DescriptorParserTests in a separate SamRockProtocol.UnitTests project - #17
Merged
rockstardev merged 3 commits intoMay 26, 2026
Conversation
…itTests project PR rockstardev#15 deleted DescriptorParserTests because the compile-time refs to SamRockProtocol.Services types triggered a JIT-time assembly load in the SamRockProtocol.Tests integration assembly, which then poisoned BTCPay's PluginManager AppDomain scan and locked out MVC controller registration for the plugin under test. Restore them in a dedicated SamRockProtocol.UnitTests project that references ONLY the plugin csproj (no BTCPayServer test fixtures). The integration test assembly (SamRockProtocol.Tests) stays disentangled - it does not reference this new project, and this new project does not reference any BTCPay test types, so the poison-via-AppDomain-load path stays closed. - New SamRockProtocol.UnitTests/SamRockProtocol.UnitTests.csproj (net8.0, xunit + xunit.runner.visualstudio + Microsoft.NET.Test.Sdk). - New SamRockProtocol.UnitTests/DescriptorParserTests.cs - 16 restored tests from c881ef2's original PR rockstardev#13 content, namespace renamed to SamRockProtocol.UnitTests. 25 distinct test cases including the [Theory] parametrizations. - SamRockProtocol.sln gets the new project. - .github/workflows/playwright.yml splits the test step into two: unit tests run first (~1s, no Docker), integration tests after. Keeps the fast unit-test feedback loop separate from the heavy ServerTester boot path.
…tegration assembly PR rockstardev#15 squash-merge only landed the AppDomain force-load removal from SharedPluginTestFixture - it did NOT land the DescriptorParserTests deletion that the PR branch carried. As a result, master still has: - SamRockProtocol.Tests/DescriptorParserTests.cs with `using SamRockProtocol.Services;` compile-time refs - a `<ProjectReference Include="..\Plugins\SamRockProtocol\..." />` in SamRockProtocol.Tests.csproj Either is enough to trigger a JIT-time load of the SamRockProtocol assembly into the integration test process's AppDomain at xunit- discovery time, before PluginManager.AddPlugins runs. PluginManager then scans AppDomain.CurrentDomain.GetAssemblies(), finds the plugin already present, registers with Loader=null, and the dedup at PluginManager.cs:204 locks out the file-based PluginLoader path. mvcBuilder.AddPluginLoader is skipped because Loader is null - MVC ApplicationParts never includes the plugin's controllers - every plugin route 404s. Master CI #26 on 14a4389 reproduces exactly this: plugin in DI list, OTP create 404 x20 attempts, test fails. Removing both the file + the ProjectReference from the integration assembly closes the AppDomain-poison path by construction. The plugin DLL is still produced by the solution build (the .sln + the new SamRockProtocol.UnitTests project both still reference it), and the "Configure plugin path for tests" workflow step still copies it to ~/.btcpayserver/Plugins/SamRockProtocol/ so the runtime PluginLoader path can load it cleanly. The new SamRockProtocol.UnitTests project (added in this PR) keeps the DescriptorParser unit tests alive in a process-isolated assembly that has no compile-time relation to SamRockProtocol.Tests, so it cannot re-poison the integration AppDomain. Validation: - Solution build clean (0 errors) - SamRockProtocol.UnitTests: 25/25 passing locally - Integration test runs via separate `dotnet test SamRockProtocol.Tests` step in CI, in its own process, with no plugin metadata reference
Hermes nit on PR rockstardev#17 review: workflow comment said unit tests don't need Docker, but YAML ordering had Docker boot before unit tests so the fast-feedback loop wasn't actually delivered. Reorder to: Run unit tests -> Start Docker containers -> Run integration tests Unit tests fail fast (~1s) without paying the Docker boot cost.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to PR #15 (master CI route-404 fix).
PR #15 deleted
SamRockProtocol.Tests/DescriptorParserTests.csbecause the compile-time refs toSamRockProtocol.Services.DescriptorParsertriggered a JIT-time assembly load in the integration-test process. That load happened BEFORE BTCPay'sPluginManager.AddPluginsran, which then scannedAppDomain.CurrentDomain.GetAssemblies(), found the plugin already there, registered it withLoader=null, and the dedup atPluginManager.cs:204locked out the file-basedPluginLoaderpath. Net effect: plugin in DI, MVCApplicationPartsnever includes the plugin's controllers, every plugin route 404s. Deleting the unit tests was the smallest patch that unblocked the master CI; restoring them in the same assembly would re-introduce the bug.This PR restores the DescriptorParser unit tests in a dedicated
SamRockProtocol.UnitTests/project that has NO compile-time reference back fromSamRockProtocol.Tests(the integration assembly), so the poison-via-AppDomain-load path stays closed.Changes:
SamRockProtocol.UnitTests/SamRockProtocol.UnitTests.csproj(net8.0, xunit + xunit.runner.visualstudio + Microsoft.NET.Test.Sdk). References ONLY the plugin csproj.SamRockProtocol.UnitTests/DescriptorParserTests.cs- 16 restored tests from c881ef2's original PR Cleanup: extract DescriptorParser + unit tests + bump to 1.1.0 #13 content, namespaceSamRockProtocol.UnitTests. 25 distinct test cases including the[Theory]parametrizations.SamRockProtocol.slngets the new project..github/workflows/playwright.ymlsplits the test step into two: unit tests run first (~1s, no Docker), integration tests after. Fast unit-test feedback stays separate from the heavyServerTesterboot path.Verifies the AppDomain-isolation by construction:
SamRockProtocol.UnitTests->SamRockProtocol(plugin) only.SamRockProtocol.Tests(integration) -> BTCPay test fixtures; does NOT referenceSamRockProtocol.UnitTests, does NOT referenceSamRockProtocolservices at compile time. The integration test only exercises the plugin via the plugin-loader path at runtime.Local: 25 unit tests passing on net8.0.